-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update snow DA cycling for the fractional grid setting. #1687
Update snow DA cycling for the fractional grid setting. #1687
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see inline comments.
sorc/link_workflow.sh
Outdated
@@ -169,6 +169,7 @@ if [[ -d "${script_dir}/gdas.cd" ]]; then | |||
cd "${top_dir}/ush" || exit 1 | |||
${LINK} "${script_dir}/gdas.cd/ush/ufsda" . | |||
${LINK} "${script_dir}/gdas.cd/ush/jediinc2fv3.py" . | |||
${LINK} "${script_dir}/gdas.cd/ush/land/gfs-land_gfsv17.yaml" . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is a yaml file being linked into ush
?
- [$(HOMEgfs)/fix/gdas/fv3jedi/fv3files/akbk$(npz).nc4, $(DATA)/fv3jedi/akbk.nc4] | ||
- [$(HOMEgfs)/fix/gdas/fv3jedi/fv3files/fmsmpp.nml, $(DATA)/fv3jedi/fmsmpp.nml] | ||
- [$(HOMEgfs)/fix/gdas/fv3jedi/fv3files/field_table_gfdl, $(DATA)/fv3jedi/field_table] | ||
- [$(HOMEgfs)/ush/gfs-land_gfsv17.yaml, $(DATA)/fv3jedi/gfs-land.yaml] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this in ush
? This should be a file in $(HOMEgfs)/parm/parm_gdas/...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can handle this case in the land_jedi_fix.yaml
itself instead of adding this yaml.
if localconf.FRAC_GRID: | ||
jedi_fix_list_path = os.path.join(self.task_config.HOMEgfs, 'parm', 'parm_gdas', 'land_jedi_fix_gfsv17.yaml') | ||
else: | ||
jedi_fix_list_path = os.path.join(self.task_config.HOMEgfs, 'parm', 'parm_gdas', 'land_jedi_fix.yaml') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do this in the land_jedi_fix.yaml
itself instead of in the python script.
.gitignore
Outdated
@@ -131,6 +131,7 @@ ush/global_chgres_driver.sh | |||
ush/global_cycle.sh | |||
ush/global_cycle_driver.sh | |||
ush/jediinc2fv3.py | |||
ush/gfs-land_gfsv17.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is a yaml in a ush
?
Thanks @aerorahul for your quick review. I did agree with all you suggestions. However, we just had a meeting and plan to handle the fractional grid issue in JEDI fv3-jedi repo directly. Therefore, I will close this PR. Thanks again. |
@aerorahul @CoryMartin-NOAA I have a concern. The current global-workflow supports both fractional grid cells and non fractional grid cells. If the above proposed changes are made in the JEDI fv3-jedi repo, the snow DA cycling will not work any more for the non fractional grid settings. |
@jiaruidong2017 |
So is this PR able to be closed then? |
As I mentioned in GDASApp's PR, the proposed changes can support both fractional and non fractional grids in the global-workflow. |
@jiaruidong2017 is that true for the model or just the DA? |
The workflow no longer supports non-fractional grid. |
@CoryMartin-NOAA I mean for snow DA only |
@jiaruidong2017 if the model doesn't support it, then there is no real need for us to support both for the snow DA |
@aerorahul Do you mean we need to remove all |
The ufs-weather-model itself does support both fractional and non-fractional grid to my knowledge. @junwang-noaa can correct if needed. |
@CoryMartin-NOAA I have no idea whether the model support the non fractional grid or not, and I will check this with @barlage |
Thanks @JessicaMeixner-NOAA for your info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FRAC_GRID is still a variable in the workflow from a search, so from what I can tell, this one line change looks good to me. But is this PR in draft waiting for a GDASApp hash?
@CoryMartin-NOAA Yes, this PR depends on the GDASApp PR #504 |
Yes, the model does support both fractional grid and non-fractional grid. |
The model does support non-fractional grids. |
@jiaruidong2017 verify that NOAA-EMC/GDASApp@4452b0a has what you need and if so, update this PR to use this commit |
Thanks @CoryMartin-NOAA and I will test the NOAA-EMC/GDASApp@4452b0a |
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Is this ready to merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good.
Assuming the hashes of |
This PR will allow users to run JEDI-based LETKFOI update for snow depth analysis w/o the fractional grids settings of GFSv17.
In previous GFS versions, the variable for total snow depth (
snwdph
) was only defined over land grid cells. In the new GFSv17 restarts, with fractional grids, thesnwdph
variable in the restarts is the average snow depth over the grid cell [ = area-weighted average of the snow depth over sea ice (snodi
) and over land (snodl
)].snodl
is the snow depth over land portion of a cell,snodi
is the snow depth over ice portion of a cell, andsnwdph
is the snow depth over the entire grid cell.This PR depend on:
NOAA-EMC/GDASApp#504
Type of change
How Has This Been Tested?
Ran a standalone landanl.sh job on Hera.
Checklist